Report will_be_retried from the retry filter's decision - #1910
Open
Enceradeira wants to merge 2 commits into
Open
Report will_be_retried from the retry filter's decision#1910Enceradeira wants to merge 2 commits into
Enceradeira wants to merge 2 commits into
Conversation
5 tasks
Enceradeira
force-pushed
the
fix/cucumber-ruby-1905-will-be-retried
branch
from
September 5, 2026 15:23
d7f731e to
6022990
Compare
The TestCaseFinished message of a scenario claims it will be retried based on --retry alone, ignoring the --retry-total circuit breaker, and is off by one. The HTML formatter relies on that flag to pick the final attempt of every scenario, so it drops or duplicates scenarios. The new scenario lists every attempt reported by the message formatter with its willBeRetried flag, for a run that hits --retry-total. See #1905.
The runner reported will_be_retried from --retry alone, ignoring the --retry-total circuit breaker of the retry filter. Once that limit was hit, the TestCaseFinished message of a failed scenario still claimed it would be retried, so the HTML formatter dropped the scenario from the report. The value was also off by one, as the runner was given the number of retries where it expected the number of attempts. A RetryPolicy, shared by the retry filter and the runner, now owns the decision. The filter asks it and records what it did, the runner asks it when reporting the outcome of a test case. Fixes #1905.
Enceradeira
force-pushed
the
fix/cucumber-ruby-1905-will-be-retried
branch
from
September 5, 2026 15:29
6022990 to
990ed6c
Compare
This was referenced Sep 5, 2026
Enceradeira
marked this pull request as ready for review
September 5, 2026 15:51
luke-hill
reviewed
Sep 8, 2026
| create_formatters | ||
|
|
||
| receiver = Test::Runner.new(@configuration.event_bus, @configuration.id_generator, Cucumber::Formatter::BacktraceFilter, @configuration.retry_attempts) | ||
| receiver = Test::Runner.new(@configuration.event_bus, @configuration.id_generator, Cucumber::Formatter::BacktraceFilter, @configuration.retry_policy) |
Contributor
There was a problem hiding this comment.
Note to self / others. We're baking in more and more complexity here by using posargs.
Given we're releasing a breaking change next (And it's already agreed to be big). I think we need to move to kwargs so we can have some mandated and some optional not reliant solely on pos
Contributor
There was a problem hiding this comment.
My pref.
Line above.
options = {} then add in each opt.
Caller takes 1 arg. options
luke-hill
reviewed
Sep 8, 2026
| # The retry filter asks it and then records what it did, the test runner asks it when it reports the outcome of a | ||
| # test case. Both have to get the same answer, so #will_be_retried? never changes the state of the policy. | ||
| class RetryPolicy | ||
| def initialize(max_retries, max_permanent_failures) |
Contributor
There was a problem hiding this comment.
Can we instead lean on our config. I believe this is available in core as well
This is not 100% "known", just a thought. I could be wrong so ignore if I am
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes #1905. Companion to cucumber/cucumber-ruby-core#345, and an alternative to #1907 / cucumber/cucumber-ruby-core#344.
With
--retry 1 --retry-total 2and three failing scenarios, the third scenario is missing from the HTML report. TheTestCaseFinishedmessage of that scenario claimswillBeRetried: true, because the flag was computed from--retryalone, but the retry filter, honouring--retry-total, never retries it. The HTML formatter hides an attempt that will be retried and waits for one that never comes.On
mainthe messages are broken in two further ways, which #1907 does not address:--retry 1willBeRetriedis nevertrue, so a retried attempt is reported as a final result and the report shows the scenario twice.TestCaseFinishedis emitted after the retry, with the started id of the retry (fixed in the core PR).This PR introduces
Cucumber::RetryPolicy, shared by the retry filter and the runner. The filter asks it whether a failed test case is retried and records what it did, the runner asks the same question when it reports the outcome.will_be_retried?has no side effects, so both get the same answer.A new scenario in
features/docs/cli/retry_failing_tests.featureasserts the message values (attempt,willBeRetried, and that everytestCaseFinishedreferences the started id of the same attempt), which no existing test did.Notes
Gemfiletemporarily takescucumber-corefrom the branch of Let a retry policy decide will_be_retried in the runner cucumber-ruby-core#345 so CI can run. To be removed once core 20 is released, together with raising the upper bound incucumber.gemspec.attemptis emitted 1-based while the messages spec says the first attempt should be 0. Pre-existing and out of scope here.Type of change
Checklist:
bundle exec rubocopreports no offenses